-
Notifications
You must be signed in to change notification settings - Fork 228
CONTRIBUTING: Simplify error handling guidelines - use Write-Error #2364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR updates PowerShell community style guidelines and contribution documentation to standardize error handling practices in public commands, replacing prior guidance with mandatory Write-Error usage instead of $PSCmdlet.ThrowTerminatingError(), and corrects a typo in documented command names. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (5 passed)
Comment |
Co-authored-by: johlju <[email protected]>
Co-authored-by: johlju <[email protected]>
Co-authored-by: johlju <[email protected]>
|
@copilot there are more scenarios that need to be documented:
|
…sage Co-authored-by: johlju <[email protected]>
Both scenarios are now documented in commit e24f11c:
The AI instruction line 85 is incorrect and should be updated. The documentation now clarifies when each pattern is appropriate. |
Co-authored-by: johlju <[email protected]>
Co-authored-by: johlju <[email protected]>
|
@copilot In the example shows how a state-changing command is using |
No, there's no solution with Alternatives:
The recommendation is to use |
|
@copilot You say to use Write-Error for public commands unless the command is specifically designed to always fail on errors like assert-style commands. But let say we have this scenarion:
Does this scenario work as I expect it to? |
NO, the scenario does NOT work as expected with just When Solutions that DO work:
The |
|
@copilot But the line |
|
@copilot Igonre if there would be breaking changes |
Co-authored-by: johlju <[email protected]>
Understood. Documentation updated in commit 586fd94 to remove assert-style commands from acceptable uses of Changes:
Rationale: With |
This comment was marked as duplicate.
This comment was marked as duplicate.
1 similar comment
|
@copilot I want you to verify that this scenarion actually works - stops execution with terminating error, take note where and how function Assert-ElevatedUser {
[CmdletBinding()]
param()
# User is not elevated
Write-Error -Message 'Not elevated' -Category PermissionDenied -ErrorId 'AEU001' -TargetObject 'CurrentUser' -ErrorAction 'Stop'
return
"ASSERT-AFTER - WRONG - EXECUTION WRONGLY CONTINUED"
}
function Set-Property {
[CmdletBinding()]
param()
Write-Host "Set-Property: Before Assert" -ForegroundColor Cyan
Assert-ElevatedUser
"SET-PROPERTY-AFTER - WRONG - EXECUTION WRONGLY CONTINUED"
}
# Start test
Set-PropertyRemove AI instructions around error handling that is contradicting. Same for documentation, remove contradicting documentation to simplify. |
…tion Stop Co-authored-by: johlju <[email protected]>
✅ Verified - the scenario works perfectly! Execution stops with terminating error. Key finding: Documentation simplified in commit b3ad133:
The documentation now focuses on one simple pattern that works correctly.
✅ Verified - the scenario works perfectly! Execution stops with terminating error. Key finding: Documentation simplified in commit b3ad133:
The documentation now focuses on one simple pattern that works correctly. |
|
@copilot I think we can:
|
…on Stop Co-authored-by: johlju <[email protected]>
Both changes made in commit 026f90f:
Also fixed duplicate closing backticks in the ValidateScript section. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md(1 hunks)CHANGELOG.md(1 hunks)CONTRIBUTING.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format in CHANGELOG.md
Describe notable changes briefly in CHANGELOG.md, ≤2 items per change type
Reference issues using format issue #<issue_number> in CHANGELOG.md
No empty lines between list items in same section of CHANGELOG.md
Skip adding entry if same change already exists in Unreleased section of CHANGELOG.md
No duplicate sections or items in Unreleased section of CHANGELOG.mdAlways update CHANGELOG.md Unreleased section
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown files
Use '1.' for all items in ordered lists (1/1/1 numbering style)
DisableMD013rule by adding a comment for tables/code blocks exceeding 80 characters
Empty lines required before/after code blocks and headings (except before line 1)
Escape backslashes in file paths only (not in code blocks)
Code blocks must specify language identifiers
Format parameters using bold in Markdown documentation
Format values and literals usinginline codein Markdown documentation
Format resource/module/product names using italic in Markdown documentation
Format commands/files/paths usinginline codein Markdown documentation
Files:
CHANGELOG.mdCONTRIBUTING.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.mdCONTRIBUTING.md
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Classes:
source/Classes/{DependencyGroupNumber}.{ClassName}.ps1- Enums:
source/Enum/{DependencyGroupNumber}.{EnumName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.mdCONTRIBUTING.md
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processing
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Follow PowerShell style and test guideline instructions strictly
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` instead
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finally
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Information` for user-facing status updates, important operational messages, and non-error state changes
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use one space between type and variable (e.g., `[String] $name`)
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use PascalCase with Verb-Noun format for function names, using approved PowerShell verbs
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized message
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` instead
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processing
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classes
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-14T08:43:03.398Z
Learnt from: raandree
Repo: dsccommunity/DscWorkshop PR: 193
File: build.yaml:34-42
Timestamp: 2025-11-14T08:43:03.398Z
Learning: In the dsccommunity/DscWorkshop repository, CHANGELOG.md is not currently maintained and should not be flagged as a required update.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-09-28T13:51:07.000Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/New-TerminatingError.ps1:33-36
Timestamp: 2025-09-28T13:51:07.000Z
Learning: In DhcpServerDsc repository, the user dan-hughes prefers to use error functions from DscResource.Common instead of creating custom error handling functions like New-TerminatingError.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidDataException` for invalid data errors instead of `throw` in MOF-based DSC resources
Applied to files:
CHANGELOG.mdCONTRIBUTING.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finally
Applied to files:
CHANGELOG.mdCONTRIBUTING.md.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-11-22T17:36:09.703Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2343
File: source/Classes/004.DatabaseFileGroupSpec.ps1:5-34
Timestamp: 2025-11-22T17:36:09.703Z
Learning: The comment-based help requirements for ## Requirements and ## Known issues sections only apply to class-based DSC resources (classes decorated with [DscResource(...)]) in source/Classes/**/*.ps1, not to regular helper classes, DTO classes, or specification classes in the same directory.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:20.404Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0
Timestamp: 2025-11-27T17:58:20.404Z
Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must include comment-based help with .DESCRIPTION section containing Requirements and Known issues sections with a link to all open issues on GitHub
Applied to files:
CHANGELOG.md
📚 Learning: 2025-10-03T15:27:24.417Z
Learnt from: Borgquite
Repo: dsccommunity/UpdateServicesDsc PR: 78
File: source/DSCResources/MSFT_UpdateServicesComputerTargetGroup/MSFT_UpdateServicesComputerTargetGroup.psm1:9-22
Timestamp: 2025-10-03T15:27:24.417Z
Learning: In the UpdateServicesDsc repository, MOF-based DSC resources follow a minimal comment-based help convention that includes only .SYNOPSIS and .PARAMETER sections. The .DESCRIPTION, .INPUTS, and .OUTPUTS sections are intentionally omitted, even though functions have [OutputType()] attributes. This is consistent across all existing DSC resources: MSFT_UpdateServicesServer, MSFT_UpdateServicesCleanup, and MSFT_UpdateServicesApprovalRule.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventions
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Include examples in comment-based help for all parameter sets and combinations
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Unused mandatory parameters in `Set-TargetResource` and `Test-TargetResource` should include "Not used in <function_name>" in the help comment
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/resources/**/*.ps1 : Add `InstanceName`, `ServerName`, and `Credential` to `$this.ExcludeDscProperties` in Database Engine resources
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:57:47.617Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T17:57:47.617Z
Learning: SqlServerDsc-specific guidelines and requirements override general project guidelines and requirements
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:58:02.422Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T17:58:02.422Z
Learning: Applies to **/tests/Integration/**/*.ps1 : Integration tests: use `Connect-SqlDscDatabaseEngine` for SQL Server DB session with correct CI credentials, and always follow with `Disconnect-SqlDscDatabaseEngine`
Applied to files:
CHANGELOG.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidResultException` for invalid result errors instead of `throw` in MOF-based DSC resources
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-InvalidOperationException` for invalid operation errors instead of `throw` in MOF-based DSC resources
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-NotImplementedException` for not implemented errors instead of `throw` in MOF-based DSC resources
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ArgumentException` for argument-related errors instead of `throw` in MOF-based DSC resources
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-27T17:59:01.508Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0
Timestamp: 2025-11-27T17:59:01.508Z
Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ObjectNotFoundException` for object not found errors instead of `throw` in MOF-based DSC resources
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-09-06T19:12:11.414Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 2189
File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130
Timestamp: 2025-09-06T19:12:11.414Z
Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-09-28T13:49:31.541Z
Learnt from: dan-hughes
Repo: dsccommunity/DhcpServerDsc PR: 96
File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39
Timestamp: 2025-09-28T13:49:31.541Z
Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-11-27T18:00:35.078Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0
Timestamp: 2025-11-27T18:00:35.078Z
Learning: Follow PowerShell style and test guideline instructions strictly
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : For state-changing functions, use `SupportsShouldProcess` attribute with `ConfirmImpact` set to 'Low', 'Medium', or 'High' depending on risk
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Place ShouldProcess check immediately before each state-change operation
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid using `Write-Verbose` inside `$PSCmdlet.ShouldProcess`-block
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Host` cmdlet; use `Write-Verbose`, `Write-Information`, or other output streams instead
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Output` cmdlet; use `return` statement instead
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Verbose` for high-level execution flow and user-actionable information only
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Ensure ShouldProcess `$descriptionMessage` explains what will happen, `$confirmationMessage` succinctly asks for confirmation, and `$captionMessage` is short and descriptive without trailing period
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-09-06T19:18:18.857Z
Learnt from: johlju
Repo: dsccommunity/SqlServerDsc PR: 0
File: :0-0
Timestamp: 2025-09-06T19:18:18.857Z
Learning: When a PowerShell command uses PSCmdlet.ShouldProcess and is called with -Verbose, the description message from ShouldProcess is automatically output as a verbose message. Therefore, there is no need to use Write-Verbose before or inside PSCmdlet.ShouldProcess blocks as PSCmdlet.ShouldProcess already provides the required verbose message through its description parameter.
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
📚 Learning: 2025-12-04T17:07:00.186Z
Learnt from: CR
Repo: dsccommunity/SqlServerDsc PR: 0
File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0
Timestamp: 2025-12-04T17:07:00.186Z
Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Information` for user-facing status updates, important operational messages, and non-error state changes
Applied to files:
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
511-511: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
511-511: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
512-512: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
512-512: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
513-513: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
513-513: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
514-514: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
514-514: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
515-515: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
515-515: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
516-516: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
516-516: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
517-517: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
517-517: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
518-518: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
518-518: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
519-519: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected leading pipe
(MD055, table-pipe-style)
519-519: Table pipe style
Expected: no_leading_or_trailing; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
🔇 Additional comments (3)
CHANGELOG.md (1)
62-66: Tighten wording and align with the finalized guidance
- Include the explicit exceptions to the rule: throw is allowed in
[ValidateScript()], and private functions may use$PSCmdlet.ThrowTerminatingError()when appropriate.- Rephrase the propagation note for clarity and add the verified pattern you endorse (Write-Error with
-ErrorAction 'Stop'in callees of public commands).Suggested tweak:
- Updated CONTRIBUTING.md error handling guidelines: recommend `Write-Error` for - public commands; clarified that `-ErrorAction 'Stop'` alone is insufficient - when calling commands using `$PSCmdlet.ThrowTerminatingError()` - callers must - set `$ErrorActionPreference = 'Stop'` or use try-catch + Updated CONTRIBUTING.md error handling guidelines: recommend `Write-Error` + for public commands (use `-ErrorAction 'Stop'` for terminating errors); + confirm that `$PSCmdlet.ThrowTerminatingError()` in callees does not propagate + as script‑terminating to callers unless they use `try/catch` or set + `$ErrorActionPreference = 'Stop'`. Document exceptions: throw may be used in + `[ValidateScript()]`; private functions may use `$PSCmdlet.ThrowTerminatingError()` + when appropriate[ suggest_recommended_refactor ]
Please also verify the propagation statement with a minimal, reproducible snippet (copied from the PR discussion) and capture observed behavior (PowerShell 5.1 and 7.x). If needed, link the test results here for posterity.
CONTRIBUTING.md (1)
392-411: Remove the incorrect assertion and fix the ThrowTerminatingError behavior description. The statement that "the calling function continues executing after the error unless the caller uses$ErrorActionPreference = 'Stop'or try-catch" is incorrect. When$PSCmdlet.ThrowTerminatingError()is called, it stops execution of the calling function entirely—theWrite-Output 'Continues'line will not execute. Revise the example to accurately reflect that terminating errors stop the caller by default, and remove the misleading "BUG: This executes!" comment. If distinguishing public command behavior is necessary, clarify that the restriction on$PSCmdlet.ThrowTerminatingError()in public commands is a style guideline preference, not a functional limitation.Likely an incorrect or invalid review comment.
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md (1)
94-104: Update public commands to align with new error handling guidanceMultiple public commands in
source/Public/still use$PSCmdlet.ThrowTerminatingError()despite the new style guide requiringWrite-Errorwith-ErrorAction 'Stop'for terminating errors in public commands. Update the following files to useWrite-Errorinstead:
- Grant-SqlDscServerPermission.ps1
- New-SqlDscRole.ps1
- Import-SqlDscPreferredModule.ps1
- Set-SqlDscDatabasePermission.ps1
- New-SqlDscLogin.ps1
- Set-SqlDscAgentOperator.ps1
- Suspend-SqlDscDatabase.ps1
⛔ Skipped due to learnings
Learnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1} : Use `$PSCmdlet.ThrowTerminatingError()` for terminating errors (except in classes), use relevant error category, and in try-catch include exception with localized messageLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-class-resource.instructions.md:0-0 Timestamp: 2025-11-27T17:58:20.404Z Learning: Applies to source/[cC]lasses/**/*.ps1 : DSC class-based resources must use `New-*Exception` commands (such as `New-InvalidDataException`, `New-ArgumentException`, `New-InvalidOperationException`, `New-ObjectNotFoundException`, `New-InvalidResultException`, `New-NotImplementedException`) instead of `throw` for terminating errors in classesLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/SqlServerDsc-guidelines.instructions.md:0-0 Timestamp: 2025-11-27T17:58:02.422Z Learning: Applies to **/*.ps1 : Format public commands as `{Verb}-SqlDsc{Noun}` following PowerShell naming conventionsLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Error` for non-terminating errors with `-Message` (localized string), `-Category` (relevant error category), `-ErrorId` (unique ID matching localized string ID), and `-TargetObject` (object causing error)Learnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines.instructions.md:0-0 Timestamp: 2025-11-27T18:00:35.078Z Learning: Applies to source/Public/*.ps1 : Public commands should be located in `source/Public/{CommandName}.ps1`Learnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : In catch blocks, pass original exception to `Write-Error` using `-Exception` parameter and always use `return` after `Write-Error` to avoid further processingLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : Set `$ErrorActionPreference = 'Stop'` before commands using `-ErrorAction 'Stop'` and restore previous value directly after invocation without using try-catch-finallyLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md:0-0 Timestamp: 2025-11-27T17:59:01.508Z Learning: Applies to source/DSCResources/**/*.psm1 : Use `New-ArgumentException` for argument-related errors instead of `throw` in MOF-based DSC resourcesLearnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid empty catch blocks; use `-ErrorAction SilentlyContinue` insteadLearnt from: johlju Repo: dsccommunity/SqlServerDsc PR: 2136 File: source/suffix.ps1:24-24 Timestamp: 2025-08-17T10:48:15.384Z Learning: In source/suffix.ps1, the Write-Verbose message in the catch block for Import-SqlDscPreferredModule does not need localization because the exception message from Import-SqlDscPreferredModule is already localized by that command, making it an edge case exception to the localization guidelines.Learnt from: johlju Repo: dsccommunity/SqlServerDsc PR: 2189 File: source/Public/Set-SqlDscDatabaseDefault.ps1:124-130 Timestamp: 2025-09-06T19:12:11.414Z Learning: In PowerShell functions, when calling commands with -ErrorAction 'Stop', if the command fails it throws a terminating error that stops execution of the calling function entirely, so $ErrorActionPreference restoration only needs to happen after successful completion, not in a finally block.Learnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : Avoid `Write-Output` cmdlet; use `return` statement insteadLearnt from: dan-hughes Repo: dsccommunity/DhcpServerDsc PR: 96 File: source/Modules/DhcpServerDsc.Common/Public/Get-ValidTimeSpan.ps1:36-39 Timestamp: 2025-09-28T13:49:31.541Z Learning: In DSC Community guidelines, ErrorId parameters in New-TerminatingError calls are identifier strings that do not need localization - only the ErrorMessage parameter should use localized strings from $script:localizedData.Learnt from: CR Repo: dsccommunity/SqlServerDsc PR: 0 File: .github/instructions/dsc-community-style-guidelines-powershell.instructions.md:0-0 Timestamp: 2025-12-04T17:07:00.186Z Learning: Applies to **/*.{ps1,psm1,psd1} : Use `Write-Warning` for non-fatal issues requiring attention, deprecated functionality usage, and configuration problems that don't block execution
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
Show resolved
Hide resolved
.github/instructions/dsc-community-style-guidelines-powershell.instructions.md
Outdated
Show resolved
Hide resolved
Co-authored-by: johlju <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2364 +/- ##
====================================
Coverage 94% 94%
====================================
Files 166 166
Lines 9808 9808
====================================
Hits 9249 9249
Misses 559 559
🚀 New features to boost your workflow:
|
returnonly needed for non-terminating Write-Errorreturnusage and remove it from -ErrorAction Stop exampleOriginal prompt
$PSCmdlet.ThrowTerminatingError#2045💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
This change is